-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[wip] SmartState analysis POC #119
base: main
Are you sure you want to change the base?
Conversation
41c138c
to
fcbc9c1
Compare
the container exposes a service that recieves the cluster credentials, runs the smartstate and will return the results as a bulk when done. Signed-off-by: borod108 <[email protected]>
fcbc9c1
to
f668069
Compare
@@ -6,8 +6,8 @@ GO_FILES := $(shell find ./ -name ".go" -not -path "./bin" -not -path "./packagi | |||
GO_CACHE := -v $${HOME}/go/migration-planner-go-cache:/opt/app-root/src/go:Z -v $${HOME}/go/migration-planner-go-cache/.cache:/opt/app-root/src/.cache:Z | |||
TIMEOUT ?= 30m | |||
VERBOSE ?= false | |||
MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/kubev2v/migration-planner-agent | |||
MIGRATION_PLANNER_API_IMAGE ?= quay.io/kubev2v/migration-planner-api | |||
MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/bodnopoz/migration-planner-agent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please keep the defaul kubev2v
MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/kubev2v/migration-planner-agent | ||
MIGRATION_PLANNER_API_IMAGE ?= quay.io/kubev2v/migration-planner-api | ||
MIGRATION_PLANNER_AGENT_IMAGE ?= quay.io/bodnopoz/migration-planner-agent | ||
MIGRATION_PLANNER_API_IMAGE ?= quay.io/bodnopoz/migration-planner-api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
id: id, | ||
jwt: jwt, | ||
config: config, | ||
healthCheckStopCh: make(chan chan any), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make a separate PR for this.
credUrl string | ||
id uuid.UUID | ||
jwt string | ||
config *config.Config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the file formatted? gopls maybe?
require 'json' | ||
require 'manageiq-smartstate' | ||
# boot.rb | ||
$LOAD_PATH << File.expand_path("/home/bodnopoz/work/manageiq-gems-pending/lib", __FILE__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess, this needs to be fixed?
@@ -152,6 +152,16 @@ func (a *Agent) start(ctx context.Context, plannerClient client.Planner) { | |||
continue | |||
} | |||
|
|||
nsc := service.NewSmartStateClient() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be moved outside the loop.
@@ -152,6 +152,16 @@ func (a *Agent) start(ctx context.Context, plannerClient client.Planner) { | |||
continue | |||
} | |||
|
|||
nsc := service.NewSmartStateClient() | |||
zap.S().Debug("trying to get smart state results") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will pollute the log with an entry that does not add value to the logs.
|
||
type SmartStateOption func(client *SmartStateClient) | ||
|
||
func NewSmartStateClient(opts ...SmartStateOption) SmartStateClient { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should return a pointer here. the methods are attached to a pointer to the type.
} | ||
} | ||
|
||
func RemoveProtocolAndRoute(rawURL string) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be private
defer resp.Body.Close() | ||
|
||
if resp.StatusCode >= 200 && resp.StatusCode < 300 { | ||
fmt.Println("Request succeeded with status:", resp.StatusCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the logger
return nil | ||
} | ||
|
||
return fmt.Errorf("request failed with status: %d", resp.StatusCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a suggestion. It is common for the last return to be successful.
return nil, fmt.Errorf("failed to decode JSON response: %w", err) | ||
} | ||
|
||
fmt.Println("Request succeeded with status:", resp.StatusCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the logger
defer resp.Body.Close() | ||
|
||
if resp.StatusCode == 202 { | ||
fmt.Println("Request in progress. Status:", resp.StatusCode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use the logger
Add collection of additional data about the cluster using ManageIQ Smart State tool. This information is very invasive, it includes accounts, installed software etc. So this is just a POC which will be modified later to avoid gathering information we do not need. Also it takes a lot of time, about 30 seconds per VM on average.